Skip to content

chore(API docs): update header of angular-patternfly API Docs site#624

Merged
jeff-phillips-18 merged 1 commit intopatternfly:masterfrom
dabeng:update-header
Sep 27, 2017
Merged

chore(API docs): update header of angular-patternfly API Docs site#624
jeff-phillips-18 merged 1 commit intopatternfly:masterfrom
dabeng:update-header

Conversation

@dabeng
Copy link
Copy Markdown
Contributor

@dabeng dabeng commented Sep 18, 2017

Hi @jeff-phillips-18 , the class "showcase" has been removed from the body tag in this PR.

@jeff-phillips-18 jeff-phillips-18 self-requested a review September 18, 2017 14:53
Copy link
Copy Markdown
Member

@dtaylor113 dtaylor113 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to limit all changes to just css under .showcase

Comment thread misc/patternfly-showcase.css Outdated
padding-top: 20px;
transition: padding-left 0.2s cubic-bezier(0.35, 0, 0.25, 1);
}
header .navbar.navbar-default {
Copy link
Copy Markdown
Member

@dtaylor113 dtaylor113 Sep 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, this will affect every <header> element. We need to just limit this to <header>'s under showcase. Ie:

.showcase > header {
   ...
}

Comment thread misc/patternfly-showcase.css Outdated
@@ -0,0 +1,537 @@
.text-overflow-pf {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are overwritting text-overflow-pf for the entire application. This needs to be limited to just .showcase

Copy link
Copy Markdown
Member

@jeff-phillips-18 jeff-phillips-18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @dtaylor113. All the CSS selectors being added should ONLY effect the header bar. Rather than using the header element for the selector, add 'showcase' class to the header then for everything below, use that to ensure no unexpected changes are made.

For the navbar, please do the same so as not to interfere with any navbar-sidebar items that could become part of the examples.

In short, let's make sure that none of the CSS being added to the examples effect anything that is not intended.

Comment thread misc/patternfly-showcase.css Outdated
.showcase-contribute i.code {
color: #852009;
}
footer {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used anywhere? I don't see any footer as part of the page.

Comment thread misc/patternfly-showcase.css Outdated
transition: padding-left 0.2s cubic-bezier(0.35, 0, 0.25, 1);
}
header .navbar.navbar-default {
background: #040404 !important;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we be using colors from the Patternfly palette?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why so many !important's?

@junezhang
Copy link
Copy Markdown
Member

junezhang commented Sep 22, 2017

@dtaylor113 @jeff-phillips-18 I've updated the CSS part. the main structure of the less is as below:
screen shot 2017-09-22 at 11 07 36 am
The detail less file is here: https://github.com/junezhang/showcase/blob/master/dist/less/showcase.less
The detail css file is here:
https://github.com/junezhang/showcase/blob/master/dist/styles/patternfly-showcase.css

For your above questions, answered as following:

  • If angular-patternfly doesn't need footer, I think we can remove it in angular-patternfly @dabeng Also if don't need contribute page, then remove .showcase-contribute.
  • the showcase css is using the same color pattern with PatternFly-org.
  • Why so many important, because some pieces of code are from PatternFly-org, after checking, I removed some of them. thanks for reminder.

@jeff-phillips-18
Copy link
Copy Markdown
Member

jeff-phillips-18 commented Sep 22, 2017

The structure of this less file is what I was looking for thanks. If the footer can not be made more specific to the showcase components, then I would not include its CSS here.

IMO, we should be using the Patternfly color palette for the showcase apps (and for Patternfly.org for that matter) but I'm OK with that being done as an overall Patternfly.org/Repo effort if others agree.

@dtaylor113
Copy link
Copy Markdown
Member

The detail less file is here: https://github.com/junezhang/showcase/blob/master/dist/less/showcase.less

Looks good, just needs to be pushed to this PR
Thanks, Dave

@dabeng
Copy link
Copy Markdown
Contributor Author

dabeng commented Sep 27, 2017

@dtaylor113 , please review again.

@jeff-phillips-18
Copy link
Copy Markdown
Member

I've reviewed per @dtaylor113 's comments. I believe they are all resolved.

@jeff-phillips-18 jeff-phillips-18 merged commit 7648876 into patternfly:master Sep 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants